-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compile rustdoc on-demand #43482
Compile rustdoc on-demand #43482
Conversation
INTERNER.intern_path(self.build.rustc_snapshot_libdir()) | ||
} else { | ||
self.sysroot(compiler) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible we should always do the above, i.e., in cargo
below, but this makes rustdoc
work in --stage 0
contexts.
src/bootstrap/doc.rs
Outdated
let mut cmd = Command::new(&rustdoc); | ||
|
||
builder.add_rustc_lib_path(compiler, &mut cmd); | ||
let mut cmd = builder.rustdoc_cmd(builder.compiler(0, build.build)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 0
here didn't change, but is somewhat suspicious. I think we may want to request a stage from the caller, who would get it from builder.top_stage
in most cases, probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah let's change this to threading through a stage and/or compiler to here. Otherwise I think that ./x.py doc
will compile rustdoc twice, once in the last stage and once in stage0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, not without the full bootstrap option enabled. ./x.py doc
requests two things: compile::Rustdoc { target_compiler: stage = 1 }
and stage = 0
. However, both of those call into the same ToolBuild
, constructing a "stage 1" rustdoc (there's not really a way to make a stage 0 rustdoc, so we just pretend that what you want is a stage1 rustdoc). I'll still add the threading code, though.
This should fix #39505 as well |
c3f1f89
to
e13f515
Compare
Unfortunately we don't currently check before passing |
src/bootstrap/tool.rs
Outdated
let build_compiler = if target_compiler.stage == 0 { | ||
target_compiler | ||
} else { | ||
builder.compiler(target_compiler.stage - 1, target_compiler.host) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I'm not sure I quite understand the minus one here, could you elaborate on why this was needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I'll add a comment as well once you're satisfied with my explanation. rustdoc
is basically just a fancy rustc
, so what we do here is mostly the same as in the compile::Assemble
step. When building rustdoc for a given stageN/bin
we want to do so with the compiler for stage(N-1)/bin
so we subtract 1 here. Before, this was done implicitly since rustdoc was built alongside the rustc
by that stage(N-1)
compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right yes makes sense to me!
Looks great! Just a few minor nits but otherwise r=me. Very easy to read/review with the recent refactoring! |
@bors r=alexcrichton |
📌 Commit 7c8debf has been approved by |
7c8debf
to
d32a8c3
Compare
@bors r=alexcrichton Missed a few cases of passing the wrong compiler, should be fixed now. |
📌 Commit d32a8c3 has been approved by |
⌛ Testing commit d32a8c3e2c8f2322c642bbd448b9c654cb68540f with merge f92049dc328cabe6d366077f9943f59f834d2124... |
💔 Test failed - status-travis |
Rustdoc is no longer compiled in every stage, alongside rustc, instead it is only compiled when requested, and generally only for the last stage.
For most tests, rustdoc isn't needed, so avoid building it.
Previously we'd build libsyntax without it when documenting and that'd cause us to recompile it when building normally for no reason.
d32a8c3
to
288658b
Compare
288658b
to
9ee877b
Compare
@bors r=alexcrichton |
📌 Commit 9ee877b has been approved by |
Compile rustdoc on-demand Fixes #43284, fixes #38318, and fixes #39505. Doesn't directly help with #42686, since we need to rebuild just as much. In fact, this hurts it, since `./x.py doc --stage 0` will now fail. I'm not sure if it did before, but with these changes it runs into the problem where we attempt to use artifacts from bootstrap rustc with a non-bootstrap rustdoc, running into version conflicts. I believe this is solvable, but leaving for a future PR. This means that rustdoc will no longer be compiled when compiling rustc, by default. However, it is still built from `./x.py build` (for hosts, but not targets, since we don't produce compiler toolchains for them) and will be built for doc tests and crate tests. After this, the recommended workflow if you want a rustdoc is: `./x.py build --stage 1 src/tools/rustdoc` which will give you a working rustdoc in `build/triple/stage1/bin/rustdoc`. Note that you can add `src/libstd` onto the command to compile libstd as well so that the rustdoc can easily compile crates in the wild. `./x.py doc --stage 1 src/libstd` will document `libstd` with a freshly built rustdoc (if necessary), and will not rebuild rustc on modifications to rustdoc. r? @alexcrichton
☀️ Test successful - status-appveyor, status-travis |
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long? ----- So, timeline for those who need to catch up: * Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io. * A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed. * However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates. * A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously. * However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences. * That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people. * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land. * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers. * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04. * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown. And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.
Fixes #43284, fixes #38318, and fixes #39505.
Doesn't directly help with #42686, since we need to rebuild just as much. In fact, this hurts it, since
./x.py doc --stage 0
will now fail. I'm not sure if it did before, but with these changes it runs into the problem where we attempt to use artifacts from bootstrap rustc with a non-bootstrap rustdoc, running into version conflicts. I believe this is solvable, but leaving for a future PR.This means that rustdoc will no longer be compiled when compiling rustc, by default. However, it is still built from
./x.py build
(for hosts, but not targets, since we don't produce compiler toolchains for them) and will be built for doc tests and crate tests.After this, the recommended workflow if you want a rustdoc is:
./x.py build --stage 1 src/tools/rustdoc
which will give you a working rustdoc inbuild/triple/stage1/bin/rustdoc
. Note that you can addsrc/libstd
onto the command to compile libstd as well so that the rustdoc can easily compile crates in the wild../x.py doc --stage 1 src/libstd
will documentlibstd
with a freshly built rustdoc (if necessary), and will not rebuild rustc on modifications to rustdoc.r? @alexcrichton